fix(rust): qualify world-owned type paths in future/stream payload vtables#1601
Conversation
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks for the PR! I'm not sure though that the fix here is the best fix, though. I would expect this to not import more types (as that can clash), but instead to update the path used to refer to a type (e.g. a missing super:: or something like that). IIRC these type paths are printed under a special mode which understands that it's in this nested module, so I think this may be relatively easily possible?
Another possible alternative is to use const _: () = { /* ... stuff ... */ }; as a scoping mechanism instead of a module. I'm not sure if this is exactly applicable though and I'd have to dig in.
Also, FWIW, I'd appreciate to leave out the AI summaries/generation here. I'm happy to work with you, but wading through reams of AI-generated text is not always easy to do.
Sure, I'll look into changing it to update the path. Also noted on the too long AI description - i'll cut it down and rewrite it. |
d071238 to
dfb20ad
Compare
dfb20ad to
bcf79e1
Compare
@alexcrichton updated the fix to extend type_path_with_name to qualify world-owned types with path_to_root(). Rewrote PR desc as well. Let me know if there's any other changes you want here |
alexcrichton
left a comment
There was a problem hiding this comment.
Looks great, thanks! As one final request, could the test move to the tests/codegen/*.wit location? That way it'll be tested under a number of modes and easily made available to test for other languages which can be beneficial
|
@alexcrichton some of the other languages break on this test - looks like go & c++. Ok to revert to just being a Rust test for now? |
|
Could exceptions for C++/Go be added as well in a similar manner as C#? |
671a1b1 to
be1da13
Compare
Oh right, that would be much better, just pushed that up will see if it builds now. I had to do the C++ one slightly differently b/c async is supposed to be rejected but this wit file compiled |
Fixes #1598.
Reworked per review: instead of injecting
usestatements into the generatedvtable{N}module, extendtype_path_with_nameto qualify world-owned types withpath_to_root(). That's""normally and"super::super::"inIdentifier::StreamOrFuturePayloadmode, so a world-leveluse t.{r};insidefuture<result<r, _>>now resolves inside the vtable scope.Regression test in
tests/codegen.rs: fails on main withcannot find type R in this scope.